-
Notifications
You must be signed in to change notification settings - Fork 34
ci: add flaky test catching workflow #617
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
c51d15a to
353cfab
Compare
Bump test-run so we can use the `--repeat` flag. Part of tarantool#531 NO_DOC=bump NO_TEST=bump
353cfab to
f63baa9
Compare
Add new ci workflow running all files changed in the PR commits multiple times in parallel by default. Can be disabled by setting the noflaky PR label. Co-authored-by: Nikita Zheleztsov <[email protected]> Fixes tarantool#531 NO_DOC=ci NO_TEST=ci
Part of tarantool#531 NO_DOC=trivial NO_TEST=trivial
d8bcd69 to
116956d
Compare
Part of tarantool#531 NO_DOC=trivial NO_TEST=trivial
Serpentian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you a lot!
| COMMAND git diff --relative=test/ --name-only | ||
| $$\{RANGE:-master..HEAD\} | ||
| -- "./*/*test.lua" | | ||
| xargs -L 128 -r ${PROJECT_SOURCE_DIR}/test/test-run.py -j -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test-run runs for me in 1 thread, the same happens on CI, the chance of catching the flakiness, when executing the tests in one thread, is near 0. The following fixes that:
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 00ab983..7da69d4 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -13,7 +13,7 @@ add_custom_target(test-flaky
COMMAND git diff --relative=test/ --name-only
$$\{RANGE:-master..HEAD\}
-- "./*/*test.lua" |
- xargs -L 128 -r ${PROJECT_SOURCE_DIR}/test/test-run.py -j -1
+ xargs -L 128 -r ${PROJECT_SOURCE_DIR}/test/test-run.py
--force --retries=0
--repeat=$$\{N_TRIALS:-32\}
--builddir=${PROJECT_BINARY_DIR}The help from test-run:
-j [JOBS], --jobs [JOBS]
Workers count.
0 means 2 x CPU count.
-1 means everything running consistently (single process).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All other workflows use test-force, which run the tests in the single process, but there, I suppose, the idea was to minimize flakiness, and I'd rather leave them as is. But the idea of the new workflow to maximize the flakiness and it's done by running the tests in all threads
| !contains(github.event.pull_request.labels.*.name, 'notest') | ||
|
|
||
| env: | ||
| TNT_RELEASE_PATH: /home/runner/tnt-release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed, you don't cache the tarantool)
| on: | ||
| pull_request: | ||
| paths: | ||
| - 'test/*/*.lua' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should end with _test.lua or .test.lua, not just lua. Otherwise, when changing e.g. luatest_helpers and not changing any tests, all of them will be started. Or am I missing smth?
| catch_flaky: | ||
| if: github.event_name == 'pull_request' && | ||
| !contains(github.event.pull_request.labels.*.name, 'noflaky') && | ||
| !contains(github.event.pull_request.labels.*.name, 'notest') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have notest at all. I would understand if other workflows paid attention to it, but they don't, so noflaky is enough
| tt version | ||
| - name: Setup luatest | ||
| run: tt rocks install luatest 1.2.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we install some specific luatest version and not the latest? The same comment in another workflow
| - name: Install tarantool | ||
| uses: tarantool/setup-tarantool@v3 | ||
| with: | ||
| tarantool-version: '3.5' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd consider using master here. Yeah, it's much slower, but then we won't have to bump the version every several months here.
Add new ci workflow running all files changed in the PR commits multiple
times in parallel by default. Can be disabled by setting the noflaky
PR label.
Co-authored-by: Nikita Zheleztsov [email protected]
Fixes #531
NO_DOC=ci
NO_TEST=ci